Skip to content

Add rustfmt.toml configuration and re-format everything + fix basic warnings#279

Merged
theomonnom merged 2 commits intolivekit:mainfrom
daniel-abramov:main
Jan 1, 2024
Merged

Add rustfmt.toml configuration and re-format everything + fix basic warnings#279
theomonnom merged 2 commits intolivekit:mainfrom
daniel-abramov:main

Conversation

@daniel-abramov
Copy link
Contributor

So that contributions from multiple people are more consistent w.r.t. code style and formatting 🙂

Note, that this one does not include all of the cargo clippy fixes since there are quite a few of them that require manual intervention. I did not want to make them part of this PR (since unlike the rest of the changes these would not be automated, hence would require a more rigorous review of whether the user-written code changes anything).

Also, cargo check still produces 3 warnings that I did not fix as they would need some minor changes:

  • RemoteTrackPublication::on_subscription_status_changed and RemoteTrackPublication::on_permission_status_changed are pub(crate) and never used within a crate. I was not sure if I'm allowed to remove them (since we don't use them) or if I need to make them pub (that would require making the rest of the things pub as well, for consistency).
  • RtcSession::publisher() is pub, but never used (not sure why it complains about it given that it is pub, perhaps it's never exported or something).

Also I added a tiny simple job to check the formatting so that we can keep it consistent.

Copy link
Member

@theomonnom theomonnom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, TIL about use_small_heuristics

@theomonnom
Copy link
Member

Thanks 🙏
I’ll try to address the other warnings

@theomonnom theomonnom merged commit 3ea84b1 into livekit:main Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants